Repository navigation
Preserve exact comparisons in fast permutation entropy - #53
raphaelvallat merged 3 commits into
Conversation
There was a problem hiding this comment.
Thanks @antonsoo for opening this PR. See AI review comments below:
The tie-handling change looks correct: <= breaks ties by position, which matches stable argsort, and the lookup tables still line up because <= and < agree whenever there are no ties. The main concern is NaN input. Removing the jitter also removed the behavior where NaN spread to the whole result, and nothing replaces it. The first two issues below were reproduced on this branch.
NaN regressions
- Order 4 crashes (inline, line 115). With a NaN, the comparisons produce bit patterns that match no ordering, so
_PE4_LOOKUPreturns-1andbincountrejects it.perm_entropy(np.array([1, np.nan, 2, 3, 5, 4, 1, 2.]), order=4)raisesValueError: 'list' argument must have no negative elements. On master it returned-0.0. - With 2D order-4 input, a NaN in one epoch changes another epoch's result (inline, line 115). A
-1key plus offsetk*24lands in the last bin of epochk-1. In one example, epoch 0 gave 4.0926 instead of 3.9698, and its probabilities sum to more than 1. - Order 3 gives a wrong answer with no warning (inline, line 101). The keys
0b010and0b101map to 0 in_PE3_LOOKUPand_PE3_REMAP, so they are counted as pattern 0. For example,[1, nan, 2, 3, 5, 4, 1, 2]returns 1.918. - Suggested fix (inline, line 91): check for NaN up front, e.g.
np.isnan(x).any()for float input, and either raise or return NaN before any keys reach the lookup tables.
Other
5. No NaN or non-finite input tests (inline, tests/test_entropy.py). The suite passes despite the issues above.
6. Out-of-date comments (inline, line 38; also line 54). The comments describing each key bit still show strict < while the code now uses <=. Anyone rebuilding the tables from these comments would bring back the tie mismatch.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #53 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 4 4
Lines 370 374 +4
Branches 62 63 +1
=========================================
+ Hits 370 374 +4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
raphaelvallat
left a comment
raphaelvallat
left a comment
There was a problem hiding this comment.
Thank you for your contribution @antonsoo ! Merging now and I'll release a new version of Antropy
The fast permutation-entropy path casts values to float64 and adds jitter to break ties. This can erase large integer differences or reorder close floating-point values. Direct comparisons on the original values with positional tie breaking match stable sorting.
The review follow-up explicitly returns NaN for an affected order 3 or 4 epoch before it can produce invalid lookup keys or contaminate another epoch's histogram. Valid epochs are processed separately; self-comparison detects NaN while preserving integer and object-dtype comparisons. Update the bit-key comments to match the comparisons.
All 32 project tests and repository Ruff checks pass. The 2,560 original recorded/transformed feature checks retain their exact result records and agree with the independent stable-sort/counting oracle. In 96 additional recorded batches with explicitly inserted missing samples, invalid epochs now return NaN and all valid neighbours agree with the reference, correcting eight contaminated neighbour values. The two independent PRs also compose locally, with 34 tests and both original workloads passing.